-
Notifications
You must be signed in to change notification settings - Fork 29.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tools: add quote-props eslint rule #14059
Conversation
Personally I'd rather have |
I think I'm -1 on this. |
I think this is a design decision, do we prefer consistent style or do we want to leave out all quotation marks that are not strictly required? |
-0, I prefer to not use quotes unless it is really needed. |
I don't mind using |
I think we have precedence for churning in order to activate an eslint rule ( @vsemozhetbyt is the king of those 👑 #12563, #13835, etc. ) IMHO
/cc @nodejs/collaborators |
I am really not sure what to prefer. I like clear minimization ( const foo = 'foo';
const obj = {
foo,
bar: 'abc'
} vs const obj = {
foo: foo,
bar: 'abc'
} as a mind tunes in for on style and then stumbles over a different one and that hampers reading. For me, the first example is OK, but I would not want to torture those who see this differently, i.e. who possibly prefer |
Since const foo = 'foo';
const obj = {
'foo': foo,
bar: 'abc'
} |
Yes, I meant we already had a discussion on a similar style issue) |
Can we get some further opinions here? |
IMO, we should only quote properties that must be quoted. Rule changes tend to make cherry-picking for releases more difficult... |
I think this just creates churn for no particular benefit: -1. It also complicates cherry-picking. |
Also -1 for the reasons listed. |
I'd prefer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also -1 for both the reasons stated above (churn) and stylistic preference. I'm making my -1 explicit - I'm fine with "as needed" as a config or not changing the config.
I'm -1 on this too. I much prefer omitting the prop name quotes when possible to do so. |
I'm -1 as as-needed and as well. |
Closing for lack of consensus |
This adds the
quote-props
rule and uses theconsistent
style. I personally would like to change that toconsistent-as-needed
but this would create quite some churn and therefore using consistent right now is probably best.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
benchmark,test,tools